Skip to content

Add Rainwater Path Diagnostic #3946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 14, 2025
Merged

Add Rainwater Path Diagnostic #3946

merged 1 commit into from
Aug 14, 2025

Conversation

erphamucsd
Copy link
Contributor

@erphamucsd erphamucsd commented Aug 12, 2025 •
edited
Loading

Purpose

Implementing rainwater path as a diagnostic variable.

To-do

Content

  • Implemented RWP in core_diagnostics.jl.
  • Add as a diagnostic for "config/model_configs/prognostic_edmfx_dycoms_rf02_column.yml".
  • I have read and checked the items on the review checklist.

@erphamucsd
Copy link
Contributor Author

Closed and reopened because the branch was renamed to better fit the naming convention. Suggestions from previous PR have been added/committed to this branch.

@erphamucsd erphamucsd marked this pull request as ready for review August 12, 2025 20:40
akshaysridhar
akshaysridhar reviewed Aug 12, 2025
View reviewed changes
@akshaysridhar
Copy link
Member

akshaysridhar commented Aug 12, 2025 •
edited
Loading

Closed and reopened because the branch was renamed to better fit the naming convention. Suggestions from previous PR have been added/committed to this branch.

@erphamucsd On the TODO block above : the pattern moving forward will rely on reducing dependency on cache variables - so, instead of precomputing q_rai we will use the specific function if necessary to get quantities from prognostic state variables. LazyBroadcast may also help with faster gpu code; you're using scratch space here so this is not strictly necessary.

@erphamucsd erphamucsd force-pushed the ep/add_rwp branch 4 times, most recently from 6da3298 to 8756940 Compare August 13, 2025 21:31
@erphamucsd
Copy link
Contributor Author

I'm not actually sure what the standard long name for rainwater path is, and I'm also not super sure about the comment/description of the variable either.

@erphamucsd erphamucsd force-pushed the ep/add_rwp branch 7 times, most recently from 6f71c2a to 2308847 Compare August 13, 2025 22:51
akshaysridhar
akshaysridhar approved these changes Aug 14, 2025
View reviewed changes
Copy link
Member

@akshaysridhar akshaysridhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @erphamucsd!

@erphamucsd erphamucsd added this pull request to the merge queue Aug 14, 2025
Merged via the queue into main with commit 72a5699 Aug 14, 2025
15 of 19 checks passed
@erphamucsd erphamucsd deleted the ep/add_rwp branch August 14, 2025 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akshaysridhar akshaysridhar akshaysridhar approved these changes

@trontrytel trontrytel Awaiting requested review from trontrytel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erphamucsd @akshaysridhar